Skip to content

Conversation

@tyx
Copy link

@tyx tyx commented Jul 8, 2019

Hi people,

As SerializerInterface, DenormalizerInterface has the same trouble.

So I just copy the code and adapt it to work with DenormalizerInterface.

@ondrejmirtes
Copy link
Member

Hi, if the code is the same it shouldn't be done with copy-pasting source code but with reusing the same class as a different DI service.

@tyx tyx force-pushed the feature/denormalizer-interface branch from 028dbc8 to d96eaa0 Compare July 8, 2019 12:21
@tyx
Copy link
Author

tyx commented Jul 8, 2019

Yes only getClass and isMethodSupported methods change.

But I'm not sure to know what you mean behind

reusing the same class as a different DI service.

You want to inject through the constructor the variable stuff ? or by inheritance and override concerned methods ?

@tyx tyx force-pushed the feature/denormalizer-interface branch from d96eaa0 to 4a4f888 Compare July 8, 2019 12:28
@ondrejmirtes
Copy link
Member

Injecting in constructor. Register it in extension.neon multiple times with different arguments.

@tyx tyx force-pushed the feature/denormalizer-interface branch 2 times, most recently from 98c6d26 to d369fe8 Compare July 8, 2019 13:31
@tyx tyx force-pushed the feature/denormalizer-interface branch from d369fe8 to 4073030 Compare July 8, 2019 14:23
@tyx
Copy link
Author

tyx commented Jul 8, 2019

Ok, here it is !

But I'm not sure to know why tests with highest dependencies fail. If anyone can help on this, please let me know.

PHPStan\Type\Symfony\EnvelopeReturnTypeExtensionTest::testAll with data set #0 ('$test1', 'array<Symfony\Component\Messe...Stamp>')
ArgumentCountError: Too few arguments to function PHPStan\Analyser\NodeScopeResolver::__construct(), 9 passed in /home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/ExtensionTestCase.php on line 55 and exactly 10 expected
/home/travis/build/phpstan/phpstan-symfony/vendor/phpstan/phpstan/src/Analyser/NodeScopeResolver.php:140
/home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/ExtensionTestCase.php:55
/home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/EnvelopeReturnTypeExtensionTest.php:21

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Jul 8, 2019 via email

@tyx
Copy link
Author

tyx commented Jul 8, 2019

Indeed here is the commit that breaks test highest deps :
phpstan/phpstan@22f904a

I updated to change ExtensionTestCase by adding true to the NodeScopeResolver constructor.

edit: unfortunately this change breaks the lowest deps.

Please let me know what is the best solution for you.

@lookyman
Copy link
Collaborator

lookyman commented Jul 8, 2019

I'll take a look tonight. We might have to do some fancy schmancy magic to accomodate for both old and new PHPStan versions..

@ondrejmirtes
Copy link
Member

You should open 0.12-dev before merging this, that will make your job easier.

@alsciende
Copy link

Any hope this will be merged soon?

@NavyCoat
Copy link

NavyCoat commented Oct 1, 2019

@tyx @lookyman what's need to be done here? Can I help?

@tyx
Copy link
Author

tyx commented Oct 2, 2019

We still need to find a way to make both world (lowest and highest deps) green. I should admit I am using my fork for now as I failed to make it green ^^

@lookyman
Copy link
Collaborator

lookyman commented Oct 2, 2019

Guys, sorry, I completely forgot about this. I will fix and merge it this weekend during a hackathon. Thank you for your patience.

@lookyman lookyman merged commit 7d34a13 into phpstan:master Oct 6, 2019
@lookyman
Copy link
Collaborator

lookyman commented Oct 6, 2019

Thanks!

@tyx tyx deleted the feature/denormalizer-interface branch October 7, 2019 07:29
@tyx
Copy link
Author

tyx commented Oct 7, 2019

Great! Thanks for your help!

@tyx
Copy link
Author

tyx commented Oct 7, 2019

I guess we are close to publish a new release ?
0.11.6...master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants